-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
implement new types of key #1
base: master
Are you sure you want to change the base?
implement new types of key #1
Conversation
Sourcery Code Quality Report❌ Merging this PR will decrease code quality in the affected files by 12.61%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
Olá |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tem algumas correções simples como nome de variável que não segue o padrão do código, alguns blocos de código um pouco confusos e algumas coisas ligadas a padrão de projetos que pode ser melhorado.
return f"00020126360014{self.merchant.globally_unique_identifier}0114{pix.mobile}520400005303986540" \ | ||
valPix = ValidatePix(pix) | ||
detect_type = valPix.detect_type_key | ||
code = detect_type() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tu pode instanciar a classe direto e já puxar o método não precisaria passar para dentro de outra variável.
Mesma coisa o code = detect_type()
tu pode passar o método já direto dentro da string, ficaria igualmente legível.
@@ -11,7 +12,11 @@ def left_zero(self, text: str): | |||
return str(len(text)).zfill(2) | |||
|
|||
def generate(self, pix: Pix): | |||
return f"00020126360014{self.merchant.globally_unique_identifier}0114{pix.mobile}520400005303986540" \ | |||
valPix = ValidatePix(pix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aqui seguir o padrão de ter lowercase (snake_case) para as variáveis dentro de funções
if len(numbers) != 11 or len(set(numbers)) == 1: | ||
return False | ||
|
||
sum_of_products = sum(a*b for a, b in zip(numbers[0:9], range(10, 1, -1))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A logica de validação de CPF ficou um pouco confusa sobre qual digito é. Coloque um comentário para ajudar a ler no futuro sobre qual é qual mais precisamente, vai ajudar bastante quando tu tem blocos de códigos parecidos
def mobile(self): | ||
if not self.pix.mobile: | ||
raise PixError("telefone nao informado") | ||
def validateCPF(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
o padrão para o nome dos métodos também segue como lowercase (snake_case). Mesmo CPF sendo "nome" o aconselhável é seguir como os demais métodos.
""" | ||
def detect_type_key(self): | ||
key = self.pix.mobile | ||
regex = re.search(r'^[a-zA-Z0-9._-]+@[a-zA-Z0-9]+\.[a-zA-Z\.a-zA-Z]{1,10}$', key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
o nome da variável pode deixar confuso durante o uso ao longo do código, talvez ser mais especifico regex_email = re....
# 0114 - Telefone | ||
|
||
code = "" | ||
if regex is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No fluxo de validação da chave ele acaba realizando uma dupla função, Descobrir qual tipo de chave é se é email, cpf, cnpj ou telefone e validar a chave, isso torna o fluxo mais difícil de entender e tb com if um dentro do outro como é esse o caso aqui.
Ai o que é interessante de se fazer, ter fluxos distintos para isso. Em um você Descobrir qual tipo de chave é e o segundo validar a chave com base no tipo se houver um match.
Podendo assim ter vários métodos um para cada tipo de chave, e ai chamar um método que identifica o tipo de chave no validade (linha 14) e chama o método corresponde para aquele tipo de chave.
else: | ||
self.pix.mobile = f"+{self.pix.mobile}" | ||
raise Exception("A chave informada não foi identificada") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Voltar o Exception aqui vai disparar um erro que não está padronizado para tratamento. Utilizar a classe PixError, ai quando houver um erro que precise ser reportado tu não corre o risco de parar o código simulando uma Exception genérica.
@@ -11,7 +12,11 @@ def left_zero(self, text: str): | |||
return str(len(text)).zfill(2) | |||
|
|||
def generate(self, pix: Pix): | |||
return f"00020126360014{self.merchant.globally_unique_identifier}0114{pix.mobile}520400005303986540" \ | |||
valPix = ValidatePix(pix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
O ValidatePix é chamado no init.py dentro do metodo is_valid()
O code tu pode passar ele utilizando a DTO Pix o parametro que a classe recebe
def mobile(self): | ||
if not self.pix.mobile: | ||
raise PixError("Chave nao informado") | ||
code = self.detect_type_key() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Em um debug do código, o método de detect_type_key executou duas vezes. Eu deduzo que isso não seja o comportamento espero
if not self.pix.mobile: | ||
raise PixError("Chave nao informado") | ||
code = self.detect_type_key() | ||
if code == '0114': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Evitar a utilização de números mágicos como esse ao longo do código.
Que legal!! |
No description provided.